Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed schema type not being deduced correctly in compute defaults #1334 #1338

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

t-moe
Copy link
Contributor

@t-moe t-moe commented Jun 27, 2019

Reasons for making this change

As described in bug report #1334, the "default" values of schemas are not correctly applied to anyOf/oneOf properties.

This PR fixes that.
anyOf/oneOf schema don't have 'type' properties sometimes, so we needed to use getSchemaType(schema) instead of schema.type in computeDefaults

Checklist

  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed.

@epicfaace
Copy link
Member

@t-moe thanks! Can you add a test to make sure this bug doesn't regress?

@t-moe t-moe force-pushed the bug/defaultsAnyOf branch from c85a5cc to 96c21c5 Compare July 1, 2019 07:27
@t-moe
Copy link
Contributor Author

t-moe commented Jul 1, 2019

@epicfaace done

test/anyOf_test.js Outdated Show resolved Hide resolved
test/oneOf_test.js Outdated Show resolved Hide resolved
Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename those tests as I suggested? Thanks

@t-moe t-moe force-pushed the bug/defaultsAnyOf branch from 96c21c5 to fd1a675 Compare July 2, 2019 11:26
test/utils_test.js Outdated Show resolved Hide resolved
@t-moe t-moe force-pushed the bug/defaultsAnyOf branch from fd1a675 to 78e16db Compare July 3, 2019 05:54
@goralight
Copy link

Is there an Eta for this to be merged into master?

@epicfaace
Copy link
Member

Nothing pending, I just forgot to check on this. Thanks for the reminder @goralight !

@epicfaace epicfaace merged commit 750f5cb into rjsf-team:master Jul 9, 2019
@goralight
Copy link

goralight commented Jul 9, 2019

@epicfaace No worries! Thank you for merging and thank you @t-moe for fixing the bug. Could we expect a release soon ? :D

@MatinF
Copy link

MatinF commented Aug 21, 2019

@epicfaace As mentioned, I'm not sure this is yet fully resolved in the latest commit. Would be great to hear your thoughts on this. It looks like the default values are rendered in the editor, but in some cases these default values are not actually part of the submitted formData.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants